Skip to content

Move ObjectPool from diskann to diskann-utils#975

Merged
arkrishn94 merged 1 commit intomainfrom
u/adkrishnan/pq-cleanup-2
Apr 25, 2026
Merged

Move ObjectPool from diskann to diskann-utils#975
arkrishn94 merged 1 commit intomainfrom
u/adkrishnan/pq-cleanup-2

Conversation

@arkrishn94
Copy link
Copy Markdown
Contributor

@arkrishn94 arkrishn94 commented Apr 24, 2026

Relocates the object pool module so that it is available to crates that depend on diskann-utils but not diskann.

Warning: This a public API breaking change.

Motivation

Implement pool-aware distance-table allocation for PQ in diskann-quantization .

Direct importers in diskann-providers, diskann-disk, and diskann-garnet are switched to use diskann_utils::object_pool directly.

Relocates the object pool module so that it is available to crates that depend on diskann-utils but not diskann (notably diskann-quantization, which will gain pool-aware distance-table allocation in a follow-up). diskann::utils::object_pool stays as a re-export for backwards compatibility.

Direct importers in diskann-providers, diskann-disk, and diskann-garnet are switched to use diskann_utils::object_pool directly. Internal diskann users continue to use the re-export.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Relocates the ObjectPool implementation from diskann into diskann-utils so pooling utilities can be used by crates that depend on diskann-utils without pulling in diskann (e.g., upcoming quantization work).

Changes:

  • Removed diskann::utils::object_pool module and updated diskann call sites to import pooling types from diskann_utils::object_pool.
  • Added diskann-utils::object_pool module (including tests) and exported it from diskann-utils’s lib.rs.
  • Updated direct importers in diskann-providers, diskann-disk, and diskann-garnet to use diskann_utils::object_pool.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
diskann/src/utils/mod.rs Stops exposing object_pool from diskann::utils (public API surface change).
diskann/src/graph/search/scratch.rs Switches AsPooled import to diskann_utils::object_pool.
diskann/src/graph/index.rs Switches ObjectPool/PooledRef import to diskann_utils::object_pool.
diskann-utils/src/object_pool.rs New pooled-object implementation + tests moved into diskann-utils.
diskann-utils/src/lib.rs Exposes the new object_pool module publicly.
diskann-providers/src/model/pq/distance/{common,dynamic,innerproduct,l2}.rs Updates pooling imports to diskann_utils.
diskann-providers/src/model/graph/provider/async_/{memory_quant_vector_provider,fast_memory_quant_vector_provider,bf_tree/quant_vector_provider}.rs Updates ObjectPool import to diskann_utils.
diskann-garnet/src/provider.rs Updates pooling imports to diskann_utils.
diskann-disk/src/search/provider/disk_provider.rs Updates pooling imports to diskann_utils.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread diskann/src/utils/mod.rs
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.42%. Comparing base (3a20042) to head (316e21f).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #975      +/-   ##
==========================================
- Coverage   89.43%   89.42%   -0.01%     
==========================================
  Files         449      449              
  Lines       83779    83779              
==========================================
- Hits        74926    74923       -3     
- Misses       8853     8856       +3     
Flag Coverage Δ
miri 89.42% <ø> (-0.01%) ⬇️
unittests 89.27% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
diskann-disk/src/search/provider/disk_provider.rs 90.89% <ø> (ø)
diskann-garnet/src/provider.rs 83.36% <ø> (ø)
...ovider/async_/fast_memory_quant_vector_provider.rs 98.46% <ø> (ø)
...ph/provider/async_/memory_quant_vector_provider.rs 98.36% <ø> (ø)
diskann-providers/src/model/pq/distance/common.rs 100.00% <ø> (ø)
diskann-providers/src/model/pq/distance/dynamic.rs 86.91% <ø> (ø)
...nn-providers/src/model/pq/distance/innerproduct.rs 100.00% <ø> (ø)
diskann-providers/src/model/pq/distance/l2.rs 100.00% <ø> (ø)
diskann-utils/src/lib.rs 100.00% <ø> (ø)
diskann-utils/src/object_pool.rs 97.38% <ø> (ø)
... and 2 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arkrishn94 arkrishn94 merged commit cb52a9f into main Apr 25, 2026
30 checks passed
@arkrishn94 arkrishn94 deleted the u/adkrishnan/pq-cleanup-2 branch April 25, 2026 02:55
arkrishn94 added a commit that referenced this pull request May 5, 2026
Bump version to 0.51.0 due to propagate changes to downstream consumers

## Breaking API changes (AI Generated)

- **`ObjectPool` moved** (#975): now lives in `diskann-utils`. Update
imports from `diskann::...::ObjectPool` → `diskann_utils::ObjectPool`.

- **`AlignedSlice` removed** (#994): the `AlignedSlice` abstraction in
`diskann-vector` is gone. Code that converted between vector
representations through `AlignedSlice` should now use the `Poly` /
`CastFromSlice` polymorphic interfaces directly (see
`diskann-vector::conversion` and `diskann-quantization::alloc::poly`).
Storage that previously held `AlignedSlice` values should hold `Poly<T,
A>` instead.

- **`AsThreadPool` generic removed** (#967): functions that previously
took `pool: impl AsThreadPool` now take `pool: &RayonThreadPool`. Pass a
borrow of an existing pool; remove the generic parameter from your call
sites.

- **`sgemm()` returns `Result`** (#997): in `diskann-linalg`, the new
signature is:

  ```rust
  pub fn sgemm(
      atranspose: Transpose, btranspose: Transpose,
      m: usize, n: usize, k: usize,
      alpha: f32, a: &[f32], b: &[f32],
      beta: Option<f32>, c: &mut [f32],
  ) -> Result<(), SgemmError>
  ```

`SgemmError` has variants `InvalidMatrixDimensions { matrix_name,
expected_rows, expected_cols, actual_len }` and `DimensionOverflow {
matrix_name, rows, cols }`. Replace previous panic-on-bad-input
assumptions with explicit handling.

- **Benchmarks are stateful** (#995): the `Benchmark` impls in
`diskann-benchmark` are no longer stateless unit structs. Each benchmark
type now has a `::new()` constructor (often holding `PhantomData<T>` or
plugin state), and registration uses an instance:

  ```rust
  // before
  benchmarks.register("name", MyBench);
  // after
  benchmarks.register("name", MyBench::<T>::new());
  ```
If you wrote a custom benchmark, give it a `new()` and register an
instance. Combined with #996, search-side benchmarks now compose
`Plugins<Provider, Phase, Strategy>` and expose builder methods like
`.search(plugin)` to register search plugins on the instance.

- **`diskann-benchmark`: `async` → `graph-index`** (#1009): the
benchmark category previously named `async` was renamed to
`graph-index`. JSON config `type` values and example file names changed
accordingly:
  - `async-build` → `graph-index-build`
  - `async-dynamic-run` → `graph-index-dynamic-run`
- and the same prefix swap for `*-pq`, `*-sq`,
`*-spherical-quantization`, etc.

Update any benchmark config files, scripts, or CI that reference the old
`async-*` names.

- **`diskann-disk` buffer alignment decoupled from `block_size`**
(#984): code that assumed I/O buffer alignment equals the disk block
size should now configure alignment explicitly.

## Non-breaking

- New cache-aware block-transposed Chamfer/MaxSim distance for f32/f16
(#863).
- A/A benchmark documentation (#974); CI publish workflow improvements
(#755, #1017); openssl bump (#973); `compute_closest_centers` allocation
reduction (#980).
- **`DistanceComputer` `'static` bound relaxed** (#1007) and **redundant
`DistanceFunction` impls removed** (#1008)


**Full Changelog**:
v0.50.1...v0.51.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants